You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
How much do the PR code changes differ from the unmodified code?
bit for bit (for physics only runs)
different at roundoff level
more substantial for sea ice bgc
Does this PR create or have dependencies on CICE or any other models?
Yes
No
Does this PR add any new test cases?
Yes
No
Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)
Yes
No, does the documentation need to be updated at a later time?
Yes
No
Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
The ice.log files with Icepack v1.5.0 with active BGC has warning messages after about 4 years indicating negative silicate. This is because silicate limitation is not implemented correctly. The fix removes an incorrect if block around the silicate limitation functions, which was mistakenly added in Icepack v1.5.0. With this fix, no warning messages were observed over the simulation (~36 years).
@eclare108213 edited to move template from comment below to top of PR.
Fixes a bug introduced in V1.5.0.
Silicate limitation should not be wrapped in an if statement for iron tracers.
nonBFB with active BGC. BFB for all else.
@apcraig: I made a PR to E3SM-Project/Icepack identical to this one -- E3SM-Project#39. Let me knowif you'd like me to submit the PR here, keep as draft, or close.
@apcraig: I made a PR to E3SM-Project/Icepack identical to this one -- E3SM-Project#39. Let me knowif you'd like me to submit the PR here, keep as draft, or close.
Since v1.5 there are only a few updates to Icepack. This is the only one that would affect E3SM -- the others are machine ports and similar things not related to the column physics, plus one other bug fix for the case calc_Tsfc = .false.. My preference would be to merge this BGC bug fix here then pull the whole batch since v1.5 over to E3SM, to keep them in sync. But we can do them in parallel if there are concerns about doing it this way. @proteanplanet
Happy to merge this. I like the idea of pull the latest Icepack into E3SM, but that's an E3SM process issue. Looks like the PR template hasn't been filled out for this PR, that's something that should probably happen.
How much do the PR code changes differ from the unmodified code?
[x ] bit for bit (for physics only runs)
different at roundoff level
[x ] more substantial for sea ice bgc
Does this PR create or have dependencies on CICE or any other models?
Yes
[ x] No
Does this PR add any new test cases?
Yes
[x ] No
Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)
Yes
[x ] No, does the documentation need to be updated at a later time?
Yes
[x ] No
Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
The fix removes an incorrect if block around the silicate limitation functions which was mistakenly added in Icepack v1.5.0.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR checklist
Short (1 sentence) summary of your PR:
Corrects bug introduced in Icepack v1.5.0 that causes negative silicate values.
See: BGC bug in Icepack v1.5.0 E3SM-Project/E3SM#7127
Developer(s):
@njeffery
Suggest PR reviewers from list in the column to the right.
Please copy the PR test results link or provide a summary of testing completed below.
Tested in a fully-coupled biogeochemistry simulation. Simulation page and comparisons with a control run are here:
https://acme-climate.atlassian.net/wiki/spaces/HESF/pages/5121212417/20250314.v3.LR.CBGC.FrazilEcosys.noSpinup.pm-cpu
How much do the PR code changes differ from the unmodified code?
Does this PR create or have dependencies on CICE or any other models?
Does this PR add any new test cases?
Is the documentation being updated? ("Documentation" includes information on the wiki or in the .rst files from doc/source/, which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.)
Please document the changes in detail, including why the changes are made. This will become part of the PR commit log.
The ice.log files with Icepack v1.5.0 with active BGC has warning messages after about 4 years indicating negative silicate. This is because silicate limitation is not implemented correctly. The fix removes an incorrect if block around the silicate limitation functions, which was mistakenly added in Icepack v1.5.0. With this fix, no warning messages were observed over the simulation (~36 years).
@eclare108213 edited to move template from comment below to top of PR.